[kotlin][client] fix encoding of individual parts of a multipart request #11911

koscejev · 2022-03-18T12:58:15Z

Fixes #11910

This propagates param.contentType field from individual API endpoint definitions to the actual ApiClient and uses it to create RequestBody for each individual part with correct Content-Type header as well as correct serialization of the individual part's body. If not specified, the default Content-Type is "application/octet-stream" for files and "application/json" for models - see https://swagger.io/docs/specification/describing-request-body/multipart-requests/. The actual serialization of individual part's body is now reusing code for serialization of the non-multipart body, so it now properly handles many cases.

^ This part was also verified on our own internal project using ok-http4 library only.

Potential questions/risks:

I'm not 100% sure about how this works for part bodies that are neither files, nor models. But since it reuses code that is handling non-multipart body, so it is consistent with that at least.
Some samples still contain old code, even though other samples were updated by the specified script? How does this work?
Should this go to master instead? I will rebase as necessary.

PR checklist

Read the contribution guidelines.
Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
Run the following to build the project and update samples:
```
./mvnw clean package 
./bin/generate-samples.sh
./bin/utils/export_docs_generators.sh
```
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
For Windows users, please run the script in Git BASH.
File the PR against the correct branch: master (5.3.0), 6.0.x
If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@jimschubert, @dr4ke616 @karismann @Zomzog @andrewemery @4brunu @yutaka0m

koscejev · 2022-03-21T11:47:57Z

Can someone explain what's failing in that build? It seems to fail for completely unrelated stuff, I don't fully understand it.

go: downloading github.com/gorilla/mux v1.7.3
?   	github.com/GIT_USER_ID/GIT_REPO_ID	[no test files]
# golang.org/x/net/http2
/home/circleci/.go_workspace/src/golang.org/x/net/http2/transport.go:417:45: undefined: os.ErrDeadlineExceeded
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.2.1:exec (go-get) on project GoGinServer: Command execution failed. Process exited with an error: 2 (Exit value: 2) -> [Help 1]

4brunu · 2022-03-21T15:12:01Z

Hey @koscejev, first of all, thanks for your PR.
One issue that I noticed in your PR is that you are creating a new file, but it's not being generated.
You need to add the reference to that new file here.

openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/KotlinClientCodegen.java

Lines 629 to 649 in 0a48976

    
           private void processJVMOkHttpLibrary(final String infrastructureFolder) { 
        
               commonJvmMultiplatformSupportingFiles(infrastructureFolder); 
        
               addSupportingSerializerAdapters(infrastructureFolder); 
        
               additionalProperties.put(JVM, true); 
        
               additionalProperties.put(JVM_OKHTTP, true); 
        
               if (JVM_OKHTTP4.equals(getLibrary())) { 
        
                   additionalProperties.put(JVM_OKHTTP4, true); 
        
               } else if (JVM_OKHTTP3.equals(getLibrary())) { 
        
                   additionalProperties.put(JVM_OKHTTP3, true); 
        
               } 
        
               supportedLibraries.put(JVM_OKHTTP, "A workaround to use the same template folder for both 'jvm-okhttp3' and 'jvm-okhttp4'."); 
        
               setLibrary(JVM_OKHTTP); 
        
               // jvm specific supporting files 
        
               supportingFiles.add(new SupportingFile("infrastructure/Errors.kt.mustache", infrastructureFolder, "Errors.kt")); 
        
               supportingFiles.add(new SupportingFile("infrastructure/ResponseExtensions.kt.mustache", infrastructureFolder, "ResponseExtensions.kt")); 
        
               supportingFiles.add(new SupportingFile("infrastructure/ApiResponse.kt.mustache", infrastructureFolder, "ApiResponse.kt")); 
        
           }

After that I will continue with the review.
And we will see if CI passes.
Thanks

koscejev · 2022-03-21T21:22:50Z

@4brunu thanks for checking this out! In the mean time I found out that array/list of files is also not handled properly as it should be individual parts with the same name in the multipart request body, not just the whole list dumped as a single part. I will handle both problems, debug on our project, and then come back. Hopefully tomorrow.

4brunu · 2022-03-21T21:29:34Z

Sure, take your time 🙂

koscejev · 2022-03-23T13:18:20Z

I see all the code checks pass, but there are still some problems with samples/docs for other generator libraries. Unfortunately, I had some problem generating those, since I'm on Windows and even with Git Bash some stuff is generated incorrectly (e.g., with backslashes in paths), so I didn't push those unrelated changes.

koscejev · 2022-03-23T13:45:16Z

I had to run ./bin/meta-codegen.sh to update a file that made some check fail. Perhaps this could be added to PR checklist?

wing328 · 2022-03-24T16:53:29Z

I've changed the target branch to the current master, which will be released as v6.0.0.

Can you please resolve the merge conflicts when you've time?

(v5.4.0 was released like a month ago)

…l parts of a multipart request

koscejev · 2022-03-25T11:16:16Z

@wing328 thanks for checking and fixing this, I was a bit clueless about how the branching works in this project. I've rebased the changes. I also found and fixed one more small bug - but I can submit it in a separate PR, if necessary.

…est (OpenAPITools#11911) * [kotlin][client] fix encoding (and Content-Type headers) of individual parts of a multipart request * [kotlin][client] fix incorrect handling of binary downloads * [kotlin][client] update samples

@mkusaka

* Add source folder variable to fastapi fix 12118 * Add generated sample files for python-fastapi fix 12118 * [python-experimental] fixes json + charset use case (#12114) * Adds code to detect json content type when charset is also set * Updates template to properly render content type, regenerates samples * Adds test_json_with_charset * Reverts version file * Fixes typo * Add example allOf with single ref (#10948) * Add example allOf with single ref * fix dart-dio-next handling of that case * Refactor without vendor extension * Regenerate newer samples * Add a sample of an enum model array in query params (#12107) * [typescript-fetch] drop support typescript under v4.0 (#12102) * [typescript-fetch] drop support typescript under v4.0 * [typescript-fetch] update docs * [typescript-fetch] update package-lock.json manually & fix test * [typescript-fetch] fix test * update samples * [dart] Remove old dio generator (to be replaced with dart-dio-next) (#12109) * remove old `dart-dio` generator which will be replaced by `dart-dio-next` in a seperate PR * remove left-over `DartJaguarClinetCodegen` class which was sunset a while ago and is unused * fix: respect configured generator URL in swagger config (#12064) * fix: respect configured generator URL in swagger config The generated OpenAPI spec does not reflect the GENERATOR_HOST which causes wrong generated code and non-functional snippets in the UI. This PR improves that by adding the relevant parts to the spec. * style: use `OpenAPI` instead of `Swagger` * refactor: make Set creation Java 8 compatible * fix: add missing import * [typescript*] drop support typescript below 4.0 (#12123) * [typescript-axios] drop support typescript below 4.0 & update samples * [typescript-axios] update package.json & package-lock.json * [typescript-node] drop support typescript below 4.0 & update samples * [typescript-nestjs] drop support typescript below 4.0 & update samples * [typescript-redux-query] drop support typescript below 4.0 & update samples * [typescript-aurelia] drop support typescript below 4.0 & update samples * [typescript-jquery] drop support typescript below 4.0 & update samples * [typescript] drop support typescript below 4.0 & update samples * Upgrade haskell-servant to latest LTS (#12092) * [C++][Qt] update petstore to 3.0 spec (#12124) * test update sampels * update samples * add file * update readme with onesignal (#12126) * [typescript-fetch] Removed functions that are unused when withoutRuntime is true. (#12101) * [typescript-fetch] remove unused function when withoutRuntimeCheks option to true * [typescript-fetch] update samples * [kotlin][client] fix encoding of individual parts of a multipart request (#11911) * [kotlin][client] fix encoding (and Content-Type headers) of individual parts of a multipart request * [kotlin][client] fix incorrect handling of binary downloads * [kotlin][client] update samples * [python-experimental] Allow response media types to omit schema (#12135) * Adds issue spec file and attemts to generate code from it * Adds missing schema definitions * Skips fromProperty invocation if the passed in schema is none in getContent * Makes MediaType.schema optional * Adds checking that the content type is in self.content * Sets ApiResponse body type as Unset if there is no schema for it * Handles schema = None case * Adds endpoint without response schema * Reverts version files * Adds test_response_without_schema * improve errorObjectType to avoid regression (#12131) * [php-slim4] Add monolog package as default logger (#12137) * Add monolog to templates * Remove default values from DI\get helper It turned out \DI\get expects only single argument, current method call doesn't throw any errors but it should be corrected anyway. * Refresh samples * Bump async from 2.6.3 to 2.6.4 in /website (#12148) Bumps [async](https://github.com/caolan/async) from 2.6.3 to 2.6.4. - [Release notes](https://github.com/caolan/async/releases) - [Changelog](https://github.com/caolan/async/blob/v2.6.4/CHANGELOG.md) - [Commits](caolan/async@v2.6.3...v2.6.4) --- updated-dependencies: - dependency-name: async dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * do not put the invalid value of the enum to a JSON structure (#12133) * [Java] Ignore return value for Java file assert classes (#12145) * Add @mkusaka to TS technical committee (#12150) * Fix documentation for users of AbstractJavaJAXRSServerCodegen (#12142) The default for `sourceFolder` in the documentation was wrong. * [Dart][client] Adjust toJson method to use '_json' instead of 'json' to avoid shadowing fields named 'json' (#12127) (#12138) * [Dart][client] Updated dart samples Co-authored-by: 0xnf <0xnf> * [dart] Support/Fix sourceFolder parameter and docs (#12113) * [dart] Support/Fix sourceFolder parameter and docs * allow passing the sourceFolder to generators * not sure how this translates to the dart2 generator due to the `part` files * fix doc generation not showing default value of CLI options * [dart] Delete old generator doc files * [Ruby] Add support for faraday 2.x (#12112) * [ruby] Add faraday 2.x support * Remove redundant params_encoder config * Memoize Faraday connection and refactor * [Java/Spring] all-of and one-of Improvements and Fixes (was #12075) (#12089) * Fix Bug in OneOfImplementorAdditionalData pulling in wrong vars to one-of-implementors. Support parentVars in order to support fluent setter with inherited properties. Squashed commit of the following: commit f945c943777a1a496d7de8fc0a188842d9efb1ac Author: Lars Uffmann <[email protected]> Date: Thu Apr 7 18:22:54 2022 +0200 Polishing commit 23ce1d0ff1faff53e85ca4362f33660962aa6a92 Author: Lars Uffmann <[email protected]> Date: Thu Apr 7 17:15:28 2022 +0200 Add JavaDoc commit fee70fde5709afa67f3aabd4f48ba496df63a884 Author: Lars Uffmann <[email protected]> Date: Thu Apr 7 17:11:17 2022 +0200 Add imports for inherited Properties commit 29509aaac51750fbd33c00a57d32cac34cbcbb90 Author: Lars Uffmann <[email protected]> Date: Thu Apr 7 13:40:36 2022 +0200 Generate Samples commit 1d19d5465137d3af712f2fd3b4ae4474c58af15e Author: Lars Uffmann <[email protected]> Date: Thu Apr 7 13:21:23 2022 +0200 SpringCodegen: Support parentVars in order to support fluent setter with inherited properties. commit 2217a77bb747d0b07ef17407a6b5dd5c624a2551 Author: Lars Uffmann <[email protected]> Date: Thu Apr 7 07:18:50 2022 +0200 Add allVars to omit list in OneOfImplementorAdditionalData commit 90499a3b0a187971bfe25deb6355c3444dcf89a7 Author: Lars Uffmann <[email protected]> Date: Wed Apr 6 16:40:23 2022 +0200 Works exactly as needed for oneOf/allOf in Java/Spring commit b6d496d772e0d0a8d87a3b8cdba8fd3ca4db7f3f Author: Lars Uffmann <[email protected]> Date: Wed Apr 6 15:16:27 2022 +0200 Debug Session: identify critical codep path commit 85722360038107f15841d5acc448d93dee513a06 Author: Lars Uffmann <[email protected]> Date: Tue Apr 5 09:56:38 2022 +0200 Add test case to reproduce issue. commit 14acc5cd974bb5260f3751015558807e2eb1a8a1 Author: Lars Uffmann <[email protected]> Date: Tue Apr 5 06:57:30 2022 +0200 Add config to reproduce the issue * Adjust indentation. Co-authored-by: Lars Uffmann <[email protected]> * [REQ][Ruby] Ruby Allow Follow Redirect (#12047) * Add `follow_location` option Implementation of #10028 * regenerated clients * set follow_location default to true * Adds UUID to python-experimental (#12153) * Adds UUID to python-exp, allows uuid models to be generated * Adds test_UUIDString uuid model test * Fixes uuid properties in python-exp, changes maps to object data type, adds uuid data type * Adds maps data type back in * Adds missing Null and AnyType definitions and adds them to python-experimental * Generator docs updated, added missing uuid, null, anytype, and object * Adds uuid support description * Docs updated * update url to travis ci * Adds not to CodegenComposedSchemas and uses it in python-exp (#12146) Updates docs * [python-experimental] fixes bug where some singleton representations raised a RecursionError (#12157) * Adds issue components and endpoint * Regenerates samples * Fixes singleton repr, removes issue components and endpoint * Removes unused endpoint * Reverts file * Adds tests of enum, boolean, and none representations * Uses super repr for singletons that ere not none, true, or false * [swift5] Abstract away URLSession (#11651) (#12110) * [Java][OkHTTP] fix empty request body handling (#12172) * Better inline model resolver to handle inline schema in array item (#12104) * better support of inline schema in array item * update tests * update samples * regenerate samples * fix allof naming, remove files * add files * update samples * update readme * fix tests * update samples * update samples * add new files * update test spec * add back tests * remove unused files * comment out python test * update js test using own spec * remove files * remove unused files * remove files * remove unused files * better handling of allOf with a single type * comment out go test * remove test_all_of_with_single_ref_single_ref_type.py * fix inline resolver, uncomment go test * [Inline model resolver] minor enhancements/refactoring (#12175) * better code format * better code format, minor refactor * [python-experimental] Fixes enum is comparison (#12176) * Fixes enum is comparison * Reverts file * [php] make ObjectSerializer::toString actually return a string (#12158) * update php samples * Allow selection of MP REST API version for MicroProfile REST client g… (#12043) * Allow selection of MP REST API version for MicroProfile REST client generation * fix typo in pom.xml * fix typo in pom.xml, update samples * add exception when incorrect MP Rest Client version is chosen * [Java][microprofile] update API test template to work with v3.0 (#12177) * update microprofile api test to work with 3.0 * minor format change * update samples * Return type for Azure funcs (#12115) * Azure func return type * Changed to Task<IActionResult<T> * Readme * update doc * Improvements to csharp-netcore-function generator (#12183) * improvements to csharp-netcore-function generator * update samples * update doc, samples * [java-micronaut] Support Optional for non-required properties (#12144) The Micronaut generator by default adds the @nullable annotation to non-required properties and allows using the Jackson JsonNullable wrapper but it is not possible to use java.util.Optional as a wrapper for optional properties. This change adds support for using the Optional wrapper for non-required properties. * update java samples * [typescript-fetch] allow initOverrides with async function (#12098) * [typescript-fetch] allow initOverrides with async function * [typescript-fetch] update samples * [typescript-fetch] refactoring initFnction apply * [typescript-fetch] update samples * [typescript-fetch] refactoring create body function * [typescript-fetch] update samples * [typescript-fetch] make interface more flexible * [typescript-fetch] update samples * [typescript-fetch] support 2.x version of typescript & update samples * [typescript-axios] update samples * [typescript-fetch] refactor: add type alias * [typescript-fetch] override with init params even if initOverrides is function * [typescript-fetch] update samples * [Wsdl] Adding cli-option for generating different versions of WSDL-files regarding Media type versioning (content negotiation) (#12206) * add wsdl version generation * add option to use specified operationId * update samples * update cli description * [python-flask] Fix return type too strict (#12190) Flask (and connexion by extension) allows the return type to be either just the body, or the body & status code, or the body & status code & headers. This commit fixes the stated `rtype` to allow the latter two cases. * update samples, docs * Fixing bug in Kotlin Client with BigDecimal default value (#12213) * Fix duplication of "Api" when structPrefix is set (#12128) This fixes a minor duplication of the word "Api", which is already part of the classname template parameter and doesn't need to be repeated when structPrefix is set. * add samples/client/petstore/kotlin-bigdecimal-default to kotlin ci tests * remove spring-mvc samples (#12222) * [Micronaut] Add option to describe response wrappers (#12186) * Minor refactor for Micronaut generators * Add support for security roles in micronaut server generator * Micronaut Server Generator refactor the x-roles String variable * Add support for Micronaut HttpResponse wrapper * Generate samples * Optimize the usage of context-path for Micronaut server * Emit default values for aspnetcore 3 value types (#11280) Fixes #10772 for aspnetcore 3+ This allows numbers to be set to zero, and booleans to be set to false. It may make sense to port this fix to the other C# generators, though it was partially fixed (for booleans only) in the netcore client in PR9042. * update samples Co-authored-by: Justin Black <[email protected]> Co-authored-by: Peter Leibiger <[email protected]> Co-authored-by: mkusaka <[email protected]> Co-authored-by: William Cheng <[email protected]> Co-authored-by: Florian Greinacher <[email protected]> Co-authored-by: Tom Bärwinkel <[email protected]> Co-authored-by: Anton Koscejev <[email protected]> Co-authored-by: Yuriy Belenko <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Hui Yu <[email protected]> Co-authored-by: Oleh Kurpiak <[email protected]> Co-authored-by: Eric Wolf <[email protected]> Co-authored-by: 0xNF <[email protected]> Co-authored-by: Yohei Kitamura <[email protected]> Co-authored-by: cachescrubber <[email protected]> Co-authored-by: Lars Uffmann <[email protected]> Co-authored-by: Connor Moore <[email protected]> Co-authored-by: leszek-s <[email protected]> Co-authored-by: fengelniederhammer <[email protected]> Co-authored-by: Andrii Serkes <[email protected]> Co-authored-by: Abrhm7786 <[email protected]> Co-authored-by: Auke Schrijnen <[email protected]> Co-authored-by: adessoDpd <[email protected]> Co-authored-by: Oliver Ford <[email protected]> Co-authored-by: Johan Sjöblom <[email protected]> Co-authored-by: Noah Fontes <[email protected]> Co-authored-by: Andriy Dmytruk <[email protected]> Co-authored-by: matt beary <[email protected]>

koscejev mentioned this pull request Mar 18, 2022

[BUG][kotlin][client][okhttp4] non-file parts are serialized as the string without Content-Type in multipart/form-data (variant of #7658 / #11289) #11910

Closed

6 tasks

koscejev force-pushed the kotlin-client-fix-multipart-parts-encoding branch from 14cae43 to 00cf05c Compare March 18, 2022 13:13

koscejev force-pushed the kotlin-client-fix-multipart-parts-encoding branch from 00cf05c to db51e63 Compare March 23, 2022 13:06

koscejev force-pushed the kotlin-client-fix-multipart-parts-encoding branch from db51e63 to ad03200 Compare March 23, 2022 13:24

wing328 changed the base branch from 5.4.x to master March 24, 2022 16:52

wing328 added Client: Kotlin Issue: Bug labels Mar 24, 2022

wing328 added this to the 6.0.0 milestone Mar 24, 2022

koscejev added 3 commits March 25, 2022 11:33

[kotlin][client] fix encoding (and Content-Type headers) of individua…

a366fa8

…l parts of a multipart request

[kotlin][client] fix incorrect handling of binary downloads

63996d3

[kotlin][client] update samples

72b110f

koscejev force-pushed the kotlin-client-fix-multipart-parts-encoding branch from 0660344 to 72b110f Compare March 25, 2022 11:14

wing328 merged commit 7851dfe into OpenAPITools:master Apr 13, 2022

4brunu mentioned this pull request Jun 3, 2022

[Kotlin][Client] replace java.nio.* to avoid crash on Android API 25 and bellow #12529

Merged

5 tasks

4brunu mentioned this pull request Sep 15, 2022

[kotlin][client] fix file upload with okhttp #13435

Merged

5 tasks

nschimmrich mentioned this pull request Oct 7, 2024

[BUG][kotlin][jvm-spring-webclient] multipart/form-data request is broken #19799

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[kotlin][client] fix encoding of individual parts of a multipart request #11911

[kotlin][client] fix encoding of individual parts of a multipart request #11911

koscejev commented Mar 18, 2022 •

edited

Loading

koscejev commented Mar 21, 2022

4brunu commented Mar 21, 2022

koscejev commented Mar 21, 2022

4brunu commented Mar 21, 2022

koscejev commented Mar 23, 2022

koscejev commented Mar 23, 2022

wing328 commented Mar 24, 2022

koscejev commented Mar 25, 2022 •

edited

Loading

[kotlin][client] fix encoding of individual parts of a multipart request #11911

[kotlin][client] fix encoding of individual parts of a multipart request #11911

Conversation

koscejev commented Mar 18, 2022 • edited Loading

PR checklist

koscejev commented Mar 21, 2022

4brunu commented Mar 21, 2022

koscejev commented Mar 21, 2022

4brunu commented Mar 21, 2022

koscejev commented Mar 23, 2022

koscejev commented Mar 23, 2022

wing328 commented Mar 24, 2022

koscejev commented Mar 25, 2022 • edited Loading

koscejev commented Mar 18, 2022 •

edited

Loading

koscejev commented Mar 25, 2022 •

edited

Loading